Skip to content
This repository has been archived by the owner on Apr 15, 2022. It is now read-only.

[WIP] More impro #8

Merged
merged 10 commits into from
May 29, 2017
Merged

[WIP] More impro #8

merged 10 commits into from
May 29, 2017

Conversation

benjaoming
Copy link
Contributor

@benjaoming benjaoming commented May 23, 2017 •
edited
Loading

Sorry, didn't think you'd be up this early so I wasn't really clear about the other PR..

I'm doing some more improvisation...

  • Tox
  • Coverage
  • Travis
  • Python 2.7, 3.4 and pypy broken
  • Not understanding why scheduler and storage don't have __init__.py files to make them packages

@aronasorman
Copy link
Collaborator

Python 2.7, 3.4 and pypy is broken (because of using enum ?).

There's a backport available here: https://pypi.python.org/pypi/enum34

Can you add that as a dependency?

@benjaoming
Copy link
Contributor Author

@aronasorman thanks for finding that, adding! Looks good, since it's pure python..

@benjaoming benjaoming mentioned this pull request May 26, 2017
Merged
@benjaoming
Copy link
Contributor Author

Tests now pass in Python 3.4

@benjaoming
Copy link
Contributor Author

Seems we now have No module named queue in Python 2.7...

@aronasorman
Copy link
Collaborator

@benjaoming hmm it's probably been renamed between python 2 vs. python 3. Let me check the name

@aronasorman
Copy link
Collaborator

Yup, it's called Queue in python 2 and queue in python 3.

@benjaoming
Copy link
Contributor Author

@aronasorman added 3 missing __init__.py files, any reason why they were missing? I don't actually understand why this was working in Py3 tests and not in Py2.. perhaps because of a weird way that find_packages works in setup.py?

@benjaoming
Copy link
Contributor Author 

>       assert isinstance(shutdown_event, threading.Event)
E       TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types

Apparently, in Py2, Event is a factory function, returning a new instance of threading._Event

@benjaoming benjaoming mentioned this pull request May 29, 2017
Merged
@benjaoming
Copy link
Contributor Author

@aronasorman sorry I can't pursue the Py2 stuff any further. The PR is growing too much :) I recommend that you use logging outputs in tests and add -s for pytest, which you can invoke outside of tox, provided that you do pip install -e . to get the latest requirements.

There are some threads/queues that never join, and then the following error that I think you'd be better at debuggin :)

✗ pytest -s tests/worker/backends/test_inmem.py
================================================================= test session starts =================================================================
platform linux2 -- Python 2.7.12, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
rootdir: /code/barbequeue, inifile:
plugins: mock-1.6.0, cov-2.5.1
collected 3 items 

tests/worker/backends/test_inmem.py .WARNING:barbequeue.worker.backends.inmem.WORKER[moon-oscar-spaghetti-spring]:Got exception for job simplejob: id() takes exactly one argument (0 given)
Exception in thread MONITOR-fanta-stairway-monkey-west:
Traceback (most recent call last):
  File "/usr/lib/python2.7/threading.py", line 801, in __bootstrap_inner
    self.run()
  File "/code/barbequeue/src/barbequeue/common/classes.py", line 94, in run
    self.main_loop(timeout=self.DEFAULT_TIMEOUT_SECONDS)
  File "/code/barbequeue/src/barbequeue/worker/backends/inmem.py", line 84, in main_loop
    self.receive_incoming_messages(timeout)
  File "/code/barbequeue/src/barbequeue/worker/backends/inmem.py", line 98, in receive_incoming_messages
    self.handle_incoming_message(msg)
  File "/code/barbequeue/src/barbequeue/worker/backends/inmem.py", line 109, in handle_incoming_message
    raise UnknownMessageError()
UnknownMessageError

@benjaoming benjaoming merged commit b4fe0e0 into learningequality:develop May 29, 2017
aronasorman
aronasorman reviewed May 31, 2017
View reviewed changes
tox.ini
deps =
-r{toxinidir}/requirements.txt
commands =
py.test --cov=src/barbequeue --color=no {posargs}

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@benjaoming benjaoming mentioned this pull request Jun 8, 2017
Closed
@aronasorman aronasorman mentioned this pull request Jul 31, 2017
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aronasorman aronasorman aronasorman left review comments

Assignees

@aronasorman aronasorman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@benjaoming @aronasorman